Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add persistent Collection builder functions #166

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Goooler
Copy link
Contributor

@Goooler Goooler commented Dec 29, 2023

Closes #137.

@Goooler

This comment was marked as outdated.

Copy link
Contributor

@qurbonzoda qurbonzoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation for the functions should be amended. Otherwise, LGTM.

core/commonMain/src/extensions.kt Outdated Show resolved Hide resolved
core/commonMain/src/extensions.kt Outdated Show resolved Hide resolved
Comment on lines +774 to +779
* The list passed as a receiver to the [builderAction] is valid only inside that function.
* Using it outside the function produces an unspecified behavior.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builders in kotlinx.collections.immutable are capable of constructing a new persistent collection multiple times. However, I'm struggling to identify a compelling use case for retaining a reference to the builder and utilizing it outside the builderAction. If we constraint the builder to be valid only within the function, could using a MutableList as the receiver be a more performance-efficient choice?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @ilya-g, WDYT?

@Goooler Goooler force-pushed the add-build-extensions branch 2 times, most recently from 62dfb51 to c90d590 Compare January 18, 2024 10:30
Comment on lines +781 to +785
@OptIn(ExperimentalTypeInference::class, ExperimentalContracts::class)
public inline fun <T> buildPersistentList(@BuilderInference builderAction: PersistentList.Builder<T>.() -> Unit): PersistentList<T> {
contract { callsInPlace(builderAction, InvocationKind.EXACTLY_ONCE) }
return persistentListOf<T>().builder().apply(builderAction).build()
}
Copy link
Contributor Author

@Goooler Goooler Feb 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Annotate @BuilderInference for better generic type support.
  2. Add InvocationKind.EXACTLY_ONCE contract to indicate the function parameter will be invoked exactly one time.

Ref https://github.com/JetBrains/kotlin/blob/3ff2bc403cb64cee273833d4dbbaf6d8c928edbc/libraries/stdlib/src/kotlin/collections/Collections.kt#L180-L187

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants